Skip to content

Add noinlineerr linter #5826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 29, 2025
Merged

Add noinlineerr linter #5826

merged 5 commits into from
May 29, 2025

Conversation

AlwxSin
Copy link
Contributor

@AlwxSin AlwxSin commented May 23, 2025

Adding noinlineerr linter.

Purpose - allow only one style of error handling.

Instead of:

if err := doSomething(); err != nil {
    return err
}

Prefer more explicit and readable:

err := doSomething()
if err != nil {
    return err
}

Copy link

boring-cyborg bot commented May 23, 2025

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented May 23, 2025

CLA assistant check
All committers have signed the CLA.

@ldez
Copy link
Member

ldez commented May 23, 2025

In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.

  • The CLA must be signed

Pull Request Description

  • It must have a link to the linter repository.
  • It must provide a short description of the linter.

Linter

  • It must not be a duplicate of another linter or a rule of a linter (the team will help to verify that).
  • It must have a valid license (AGPL is not allowed), and the file must contain the required information by the license, ex: author, year, etc.
  • It must use Go version <= 1.23.0
  • The linter repository must have a CI and tests.
  • It must use go/analysis.
  • It must have a valid tag, ex: v1.0.0, v0.1.0.
  • It must not contain init().
  • It must not contain panic().
  • It must not contain log.Fatal(), os.Exit(), or similar.
  • It must not modify the AST.
  • It must not have false positives/negatives (the team will help to verify that).
  • It must have tests inside golangci-lint.

The Linter Tests Inside Golangci-lint

  • They must have at least one std lib import.
  • They must have integration tests without configuration (default).
  • They must have integration tests with configuration (if the linter has a configuration).

.golangci.next.reference.yml

  • The file .golangci.next.reference.yml must be updated.
  • The file .golangci.reference.yml must NOT be edited.
  • The linter must be added to the lists of available linters (alphabetical case-insensitive order).
    • enable and disable options
  • If the linter has a configuration, the exhaustive configuration of the linter must be added (alphabetical case-insensitive order)
    • The values must be different from the default ones.
    • The default values must be defined in a comment.
    • The option must have a short description.

Others Requirements

  • The files (tests and linter) inside golangci-lint must have the same name as the linter.
  • The .golangci.yml of golangci-lint itself must not be edited and the linter must not be added to this file.
  • The linters must be sorted in the alphabetical order (case-insensitive) in the lintersdb/builder_linter.go and .golangci.next.reference.yml.
  • The load mode (WithLoadMode(...)):
    • if the linter uses goanalysis.LoadModeSyntax -> no WithLoadForGoAnalysis() in lintersdb/builder_linter.go
    • if the linter uses goanalysis.LoadModeTypesInfo, it requires WithLoadForGoAnalysis() in lintersdb/builder_linter.go
  • The version in WithSince(...) must be the next minor version (v2.X.0) of golangci-lint.
  • WithURL() must contain the URL of the repository.

Recommendations

  • The file jsonschema/golangci.next.jsonschema.json should be updated.
  • The file jsonschema/golangci.jsonschema.json must NOT be edited.
  • The linter repository should have a readme and linting.
  • The linter should be published as a binary (useful for diagnosing bug origins).
  • The linter repository should have a .gitignore (IDE files, binaries, OS files, etc. should not be committed)
  • A tag should never be recreated.
  • use main as the default branch name.

The golangci-lint team will edit this comment to check the boxes before and during the review.

The code review will start after the completion of those checkboxes (except for the specific items that the team will help to verify).

The reviews should be addressed as commits (no squash).

If the author of the PR is a member of the golangci-lint team, he should not edit this message.

This checklist does not imply that we will accept the linter.

@ldez ldez self-requested a review May 23, 2025 11:45
@ldez ldez added linter: new Support new linter feedback required Requires additional feedback labels May 23, 2025
@AlwxSin
Copy link
Contributor Author

AlwxSin commented May 23, 2025

@ldez thank you for checklist.

Fixed:

  • It must use Go version <= 1.23.0
  • They must have at least one std lib import.
  • The version in WithSince(...) must be the next minor version (v2.X.0) of golangci-lint.
  • added .gitignore

Quick question:

The files (tests and linter) inside golangci-lint must have the same name as the linter.

I thought they already do – did I miss something? Could you clarify what exactly should be renamed or matched?

@ldez ldez removed the feedback required Requires additional feedback label May 23, 2025
@ldez
Copy link
Member

ldez commented May 23, 2025

I detected at least one false-positive:

func _() {
	if ok, _ := strconv.ParseBool("1"); ok {
		fmt.Println("ok")
	}
}

The linter is very limited: it only handles explicit error type references, so false negatives can happen.

@ldez
Copy link
Member

ldez commented May 23, 2025

Even if I don't like inlined if (so this linter is interesting to me), I think the implementation of the linter is too naive.

I don't know what should exactly be implemented, but I think the topic needs to be approached from a broader perspective, maybe with options and more.

@AlwxSin
Copy link
Contributor Author

AlwxSin commented May 23, 2025

@ldez Thanks for the feedback!

Do you have any specific ideas in mind?

Some thoughts I had:

Adding an option to choose the preferred style (though I’m not sure if it makes sense to only prefer the inline variant).

Adding autofix support — though that’s a bit more complex for me at the moment and would need some digging in.

Happy to hear any other suggestions you might have!

@AlwxSin
Copy link
Contributor Author

AlwxSin commented May 26, 2025

@ldez Also, just to clarify — did you mean adding an option like “Check for blank identifiers” (e.g. _ = ...)? Or was it something else?

@AlwxSin
Copy link
Contributor Author

AlwxSin commented May 26, 2025

Hey! 👋 Thanks again for the feedback — I’ve addressed most of the points in the latest update. Here’s what’s changed:

Changes since last review:

Proper type check for error:
Now using types.AssignableTo(...) instead of naive string matching — this catches aliases and other error-compatible types.

Auto-fix support:
For simple cases like if err := foo(); err != nil {} — the linter now suggests and applies a fix.

Avoids false positives:
No more warnings for _ assignments or irrelevant Init cases.

Question: Options?

Right now the linter enforces a strict rule (any inline err := ... triggers a warning).
Do you think it makes sense to introduce config options for more flexibility?

Possible ideas:

allow-inline-in-tests: true
Ignore inline error handling in _test.go files.

allow-multi-assign: true
Allow if x, err := foo(); err != nil {} (only warn on single err := cases).

Let me know if you think this is worth exploring, or if the linter should stay strict and opinionated.

@ldez ldez added the blocked Need's direct action from maintainer label May 26, 2025
@ldez
Copy link
Member

ldez commented May 28, 2025

The proposed options are not what I expected.

  • allow-inline-in-tests will not be used by golangci-lint because this must be handled inside the golangci-lint exclusions and not a the linter level.
  • allow-multi-assign I don't see why this should be a specific topic.

I'm talking about a broader perspective of inlined if, not to the restricted scope of errors.

@AlwxSin
Copy link
Contributor Author

AlwxSin commented May 28, 2025

About the broader perspective — I get your point, but I’d prefer to keep the scope limited to err := ... cases for now.
This is where most readability issues tend to show up in practice, and it matches the linter's intent (and name — noinlineerr).

I’ve often seen inline error handling being overused or misused, whereas inline Init with other values is way less common —
so I focused on the pattern that was actually causing pain in the codebases I work with.

That said, building a separate linter to cover all inline Init statements (if x := ...; cond {}) could definitely be interesting —
but that feels like a broader concern and a different project altogether.

@AlwxSin AlwxSin force-pushed the noinlineerr-linter branch from 6c44991 to ae87644 Compare May 28, 2025 16:31
@ldez
Copy link
Member

ldez commented May 28, 2025

ok, so we agree that if a new linter is added that handles the whole topic of inlined if, your linter will be deprecated inside golangci-lint and removed.

If we agree on that, we can accept this linter.

@ldez ldez added feedback required Requires additional feedback and removed blocked Need's direct action from maintainer labels May 28, 2025
@AlwxSin
Copy link
Contributor Author

AlwxSin commented May 29, 2025

Totally agree

@ldez ldez removed the feedback required Requires additional feedback label May 29, 2025
@ldez ldez force-pushed the noinlineerr-linter branch from d3987b5 to 973d987 Compare May 29, 2025 15:10
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ldez ldez added this to the v2-unreleased milestone May 29, 2025
@ldez ldez merged commit f54365b into golangci:main May 29, 2025
18 checks passed
@AlwxSin AlwxSin deleted the noinlineerr-linter branch May 30, 2025 10:48
@ldez ldez modified the milestones: v2-unreleased, v2.2 Jun 28, 2025
@G-Rath
Copy link
Contributor

G-Rath commented Jul 1, 2025

@ldez fyi it looks like this linter is not listed in https://golangci-lint.run/usage/linters/

@ldez
Copy link
Member

ldez commented Jul 1, 2025

@G-Rath refresh the page: https://golangci-lint.run/usage/linters/#noinlineerr

alaudaa-renovate bot added a commit to AlaudaDevops/golangci-lint that referenced this pull request Aug 5, 2025
* build(deps): bump github.com/go-critic/go-critic from 0.12.0 to 0.13.0 (golangci#5579)

* build(deps): bump mvdan.cc/unparam to HEAD (golangci#5584)

* build(deps): bump github.com/timakin/bodyclose from ed6a65f985e3 to 1db5c5ca4d67 (golangci#5585)

* docs: improve Docker example with cache reuse (golangci#5586)

* docs: add a migration guide for VSCode integration (golangci#5587)

* build(deps): bump github.com/ckaznocha/intrange from 0.3.0 to 0.3.1 (golangci#5589)

* feat: add option stdin for fmt command (golangci#5588)

* build(deps): bump github.com/daixiang0/gci from 0.13.5 to 0.13.6 (golangci#5592)

* fix: funlen ignore-comments (golangci#5594)

* docs: cleaning

* chore: prepare release

* docs: update GitHub Action assets (golangci#5595)

* docs: fix displaying GoLand logo on a dark theme (golangci#5598)

* fix: validate version before configuration (golangci#5599)

* fix: copy golines settings during linter settings load (golangci#5607)

* fix: forbidigo migration (golangci#5606)

* docs: add example of the field version (golangci#5609)

* chore: prepare release

* docs: update GitHub Action assets (golangci#5611)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: adaptive stargazers image (golangci#5613)

* fix: enable formatters with flags (golangci#5620)

* docs: add section about flags migration (golangci#5621)

* docs: replace 'disable-all' with 'default: none' (golangci#5622)

* docs: update reference comment for errchkjson (golangci#5623)

* fix: formatter validation message (golangci#5624)

* docs: info about the logo

* docs: fix flags example

* docs: fix flags example

* docs: clean old configuration examples (golangci#5626)

* fix: use absolute filepath inside base rule source (golangci#5629)

* chore: prepare release

* docs: update GitHub Action assets (golangci#5634)

* docs: add version field to configuration sample (golangci#5632)

* docs: remove golint in nolint example (golangci#5635)

* build(deps): bump github.com/kunwardeep/paralleltest from 1.0.10 to 1.0.13 (golangci#5636)

* docs: fix config reference for formatters (golangci#5638)

* docs: improve version documentation (golangci#5639)

* docs: fix settings examples (golangci#5643)

* build(deps): bump tar-fs and puppeteer in /docs (golangci#5653)

* build(deps): bump github.com/ghostiam/protogetter from 0.3.12 to 0.3.13 (golangci#5658)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/Antonboom/testifylint from 1.6.0 to 1.6.1 (golangci#5654)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: cleanup config reference (golangci#5659)

* dev: rewrite and simplify thanks page generation (golangci#5662)

* docs: remove tenv linter settings (golangci#5665)

* docs: improve default exclusions render (golangci#5661)

* docs: fix `go get -tool` command (golangci#5669)

* fix: gocritic importshadow checker (golangci#5673)

* build(deps): bump the linter-testdata group across 2 directories with 4 updates (golangci#5676)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: update demo (golangci#5677)

* build(deps): bump github.com/alexkohler/nakedret/v2 from 2.0.5 to 2.0.6 (golangci#5681)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.2 to 4.25.3 (golangci#5680)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: add link to issues related to some integrations (golangci#5682)

* build(deps): bump github.com/Crocmagnon/fatcontext from 0.7.1 to 0.7.2 (golangci#5685)

* feat: add config path placeholder (golangci#5650)

* feat: add an option to display absolute paths (golangci#5651)

* feat: colored diff for fmt command (golangci#5652)

* build(deps): bump github.com/tomarrell/wrapcheck/v2 from 2.10.0 to 2.11.0 (golangci#5656)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/kunwardeep/paralleltest from 1.0.13 to 1.0.14 (golangci#5657)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/mgechev/revive from 1.7.0 to 1.8.0 (golangci#5663)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/polyfloyd/go-errorlint from 1.7.1 to 1.8.0 (golangci#5686)

* feat: add warn-unused option for fmt command (golangci#5668)

* build(deps): bump github.com/firefart/nonamedreturns from 1.0.5 to 1.0.6 (golangci#5687)

* build(deps): bump github.com/bombsimon/wsl/v4 from 4.6.0 to 4.7.0 (golangci#5689)

* build(deps): bump github.com/alecthomas/chroma/v2 from 2.15.0 to 2.16.0 (golangci#5690)

* build(deps): bump go-simpler.org/sloglint from 0.9.0 to 0.10.0 (golangci#5688)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump dependencies in the linter-testdata group (golangci#5691)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: add `redundant-test-main-exit` to `revive` rules (golangci#5692)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/alingse/nilnesserr from 0.1.2 to 0.2.0 (golangci#5693)

* build(deps): bump github.com/securego/gosec/v2 from 2.22.2 to 2.22.3 (golangci#5694)

Co-authored-by: Fernandez Ludovic <[email protected]>

* fix: memory leaks when using go1.(N) with golangci-lint built with with go1.(N-1) (golangci#5695)

* build(deps): bump go-simpler.org/sloglint from 0.10.0 to 0.10.1 (golangci#5696)

* docs: explicitly describe that the `migrate` command automatically migrate `linters.presets` (golangci#5697)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump golang.org/x/sys from 0.31.0 to 0.32.0 (golangci#5699)

* build(deps): bump go-simpler.org/sloglint from 0.10.1 to 0.11.0 (golangci#5698)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump golang.org/x/oauth2 from 0.28.0 to 0.29.0 in /scripts/gen_github_action_config in the scripts group (golangci#5704)

* feat: add golangci-lint-fmt pre-commit hook (golangci#5705)

* build(deps): bump golang.org/x/tools from 0.31.0 to 0.32.0 (golangci#5708)

* build(deps): bump github.com/butuzov/ireturn from 0.3.1 to 0.4.0 (golangci#5710)

* build(deps): bump github.com/pelletier/go-toml/v2 from 2.2.3 to 2.2.4 (golangci#5711)

* docs: update section about vscode integration (golangci#5706)

Co-authored-by: logica0419 <[email protected]>

* build(deps): bump github.com/jgautheron/goconst from 1.7.1 to 1.8.1 (golangci#5712)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/golangci/unconvert to HEAD (golangci#5713)

* build(deps): bump github.com/timonwong/loggercheck from 0.10.1 to 0.11.0 (golangci#5715)

* govet: add `httpmux` analyzer (golangci#5717)

* dev: update JSON schema

* build(deps): bump github.com/mgechev/revive from 1.8.0 to 1.9.0 (golangci#5721)

* Add funcorder linter (golangci#5630)

* chore: prepare release

* docs: update GitHub Action assets (golangci#5723)

* docs: update documentation assets (golangci#5722)

Co-authored-by: Fernandez Ludovic <[email protected]>

* chore: downgrade goreleaser to v2.8.1

Related to a regression inside v2.8.2

* chore: prepare release

* docs: update changelog

* docs: update GitHub Action assets (golangci#5724)

* build(deps): bump github.com/ghostiam/protogetter from 0.3.13 to 0.3.14 (golangci#5727)

* build(deps): bump mvdan.cc/gofumpt from 0.7.0 to 0.8.0 (golangci#5728)

* build(deps): bump github.com/ldez/exptostd from 0.4.2 to 0.4.3 (golangci#5730)

* build(deps): bump github.com/ldez/usetesting from 0.4.2 to 0.4.3 (golangci#5729)

* dev: simplify mnd implementation (golangci#5731)

* build(deps): bump github.com/ghostiam/protogetter from 0.3.14 to 0.3.15 (golangci#5732)

* docs: fix default value of linters.exclusions.generated (golangci#5735)

* chore: prepare release

* docs: update GitHub Action assets (golangci#5738)

* docs: update section about GoLand integration (golangci#5740)

* fix: order of staticcheck settings during migration (golangci#5741)

Co-authored-by: Fernandez Ludovic <[email protected]>

* fix: add go.mod hash to the cache salt (golangci#5739)

* dev: disable check-generated job (golangci#5742)

* fix: related information position (golangci#5746)

* docs: GoLand IDE only support v1 for now (golangci#5750)

* fix: convert uint as pointer of uint for the migration (golangci#5755)

* build(deps): bump go.augendre.info/fatcontext from 0.7.2 to 0.8.0 (golangci#5757)

* docs: GoLand IDE support v2 (golangci#5758)

* chore: prepare release

* docs: update GitHub Action assets (golangci#5762)

* chore: prepare release (golangci#5763)

* docs: update GitHub Action assets (golangci#5764)

* chore: prepare release (golangci#5765)

* docs: update GitHub Action assets (golangci#5766)

* docs: add note about golangci-lint v2 integration in VS Code (golangci#5768)

* build(deps): bump go-simpler.org/musttag from 0.13.0 to 0.13.1 (golangci#5769)

* build(deps): bump github.com/alecthomas/chroma/v2 from 2.16.0 to 2.17.0 (golangci#5772)

* build(deps): bump github.com/tetafro/godot from 1.5.0 to 1.5.1 (golangci#5770)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump base-x from 3.0.9 to 3.0.11 in /docs (golangci#5776)

* build(deps): bump the linter-testdata group across 2 directories with 2 updates (golangci#5777)

* build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.3 to 4.25.4 (golangci#5778)

* build(deps): bump github.com/alecthomas/chroma/v2 from 2.17.0 to 2.17.2 (golangci#5779)

* chore: prepare release

* docs: update GitHub Action assets

* build(deps): bump github.com/manuelarte/funcorder from 0.2.1 to 0.3.0 (golangci#5743)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/sonatard/noctx from 0.1.0 to 0.3.3 (golangci#5771)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump golangci/golangci-lint-action from 7.0.0 to 8.0.0 in the github-actions group (golangci#5780)

* build(deps): bump golang.org/x/oauth2 from 0.29.0 to 0.30.0 in /scripts/gen_github_action_config in the scripts group (golangci#5781)

* build(deps): bump github.com/jjti/go-spancheck from 0.6.4 to 0.6.5 (golangci#5784)

* build(deps): bump golang.org/x/sys from 0.32.0 to 0.33.0 (golangci#5785)

* build(deps): bump golang.org/x/tools from 0.32.0 to 0.33.0 (golangci#5786)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/ashanbrown/makezero from 1.2.0 to 2.0.1 (golangci#5782)

* build(deps): bump github.com/ashanbrown/forbidigo from 1.6.0 to 2.1.0 (golangci#5783)

* build(deps): bump github.com/securego/gosec/v2 from 2.22.3 to 2.22.4 (golangci#5788)

* build(deps): bump github.com/manuelarte/funcorder from 0.3.0 to 0.5.0 (golangci#5792)

* Add swaggo/swag formatter (golangci#5749)

Co-authored-by: Fernandez Ludovic <[email protected]>

* feat: add arangolint linter (golangci#5718)

Co-authored-by: Fernandez Ludovic <[email protected]>

* feat: add embeddedstructfieldcheck linter (golangci#5761)

Co-authored-by: Fernandez Ludovic <[email protected]>

* fix: exclusions path-expect (golangci#5798)

* dev: simplify linter constructors (golangci#5796)

* dev: remove AUR publishing (golangci#5799)

* dev: restore aur-bin (golangci#5803)

* chore: remove dead code (golangci#5801)

* errcheck: add verbose option (golangci#5802)

* docs: fix revive defer rule configuration (golangci#5804)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/Abirdcfly/dupword from 0.1.3 to 0.1.4 (golangci#5809)

* build(deps): bump github.com/uudashr/iface from 1.3.1 to 1.3.2 (golangci#5810)

* build(deps): bump github.com/alecthomas/chroma/v2 from 2.17.2 to 2.18.0 (golangci#5812)

* build(deps): bump github.com/manuelarte/embeddedstructfieldcheck from 0.2.1 to 0.3.0 (golangci#5811)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/golangci/misspell from 0.6.0 to 0.7.0 (golangci#5813)

* build(deps): bump the linter-testdata group across 2 directories with 2 updates (golangci#5814)

Co-authored-by: Fernandez Ludovic <[email protected]>

* dev: consistent version (golangci#5817)

* docs: add description for linters.default sets (golangci#5818)

* build(deps): bump github.com/uudashr/iface from 1.3.2 to 1.4.0 (golangci#5820)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: improve typecheck FAQ (golangci#5821)

Co-authored-by: Simon Sawert <[email protected]>

* docs: note about plugin system (golangci#5822)

* build(deps): bump github.com/jgautheron/goconst from 1.8.1 to 1.8.2 (golangci#5825)

* fix: write the input to stdout when using stdin and there are no changes (golangci#5827)

* build(deps): bump github.com/santhosh-tekuri/jsonschema/v6 from 6.0.1 to 6.0.2 (golangci#5829)

* build(deps): bump github.com/sashamelentyev/usestdlibvars from 1.28.0 to 1.29.0 (golangci#5828)

Co-authored-by: Fernandez Ludovic <[email protected]>

* fix: error message when trying to migrate a migrated config (golangci#5836)

Co-authored-by: Fernandez Ludovic <[email protected]>

* fix: formatters CLI flags help message (golangci#5835)

* build(deps): bump github.com/golangci/plugin-module-register from 0.1.1 to 0.1.2 (golangci#5838)

* build(deps): bump github.com/mgechev/revive from 1.9.0 to 1.10.0 (golangci#5837)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/Abirdcfly/dupword from 0.1.4 to 0.1.5 (golangci#5839)

* chore: improve pull request template

* build(deps): bump github.com/Abirdcfly/dupword from 0.1.5 to 0.1.6 (golangci#5841)

* Add noinlineerr linter (golangci#5826)

* dev: some simplifications (golangci#5843)

* dev: improve comments (golangci#5846)

* build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.4 to 4.25.5 (golangci#5849)

* build(deps): bump the linter-testdata group across 4 directories with 4 updates (golangci#5850)

* docs: improve module plugin explanation (golangci#5851)

* dev: display list of staticcheck checks in debug (golangci#5853)

* dev: correct debug log for default rules in revive (golangci#5852)

* dev: use slice of pointers (golangci#5854)

* dev: handle linter major versions (golangci#5848)

* feat: deprecate print-resources-usage flag (golangci#5860)

* docs: add a note about `GL_DEBUG=staticcheck` (golangci#5861)

* fix: deduplicate typecheck errors (golangci#5864)

* build(deps): bump golang.org/x/mod from 0.24.0 to 0.25.0 (golangci#5868)

* build(deps): bump golang.org/x/tools from 0.33.0 to 0.34.0 (golangci#5867)

* build(deps): bump github.com/ldez/gomoddirectives from 0.6.1 to 0.7.0 (golangci#5869)

* dev: log level colors (golangci#5866)

* docs: update gomoddirectives configuration

* build(deps): bump github.com/ldez/exptostd from 0.4.3 to 0.4.4 (golangci#5876)

* build(deps): bump github.com/ldez/usetesting from 0.4.3 to 0.5.0 (golangci#5877)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump brace-expansion from 1.1.11 to 1.1.12 in /docs (golangci#5878)

* build(deps): bump github.com/securego/gosec/v2 from 2.22.4 to 2.22.5 (golangci#5880)

* fix: typecheck memory leak (golangci#5884)

* dev: remove useless condition

* docs: update migration guide with --print-issued-lines (golangci#5886)

* fix: stop the analysis after the first package analysis error (golangci#5885)

* build(deps): bump github.com/go-viper/mapstructure/v2 from 2.2.1 to 2.3.0 (golangci#5888)

* build(deps): bump brace-expansion from 1.1.11 to 1.1.12 in /.github/peril (golangci#5889)

* fix: formats consistently the code with gci (golangci#5893)

* build(deps): bump github.com/sonatard/noctx from 0.3.3 to 0.3.4 (golangci#5895)

Co-authored-by: Fernandez Ludovic <[email protected]>

* fix: unique version per custom build (golangci#5896)

* build(deps): bump github.com/bombsimon/wsl/v5 from 4.7.0 to 5.0.0 (golangci#5900)

Co-authored-by: Fernandez Ludovic <[email protected]>

* dev: add suggested migration for linter configuration (golangci#5902)

* chore: prepare release

* docs: update documentation assets (golangci#5903)

Co-authored-by: Fernandez Ludovic <[email protected]>

* varnamelen: fix configuration (golangci#5904)

* chore: prepare release

* docs: update GitHub Action assets

* dev: fix GitHub Action assets generation (golangci#5906)

* build(deps): bump the linter-testdata group across 2 directories with 3 updates (golangci#5908)

* docs: add the command to check the Go version used to build (golangci#5913)

* build(deps): bump github.com/alecthomas/chroma/v2 from 2.18.0 to 2.19.0 (golangci#5914)

* build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.5 to 4.25.6 (golangci#5918)

* godot: add noinline value into the JSONSchema (golangci#5922)

* docs: fix tutorial URL (golangci#5925)

* build(deps): bump golang.org/x/mod from 0.25.0 to 0.26.0 (golangci#5927)

* build(deps): bump github.com/AlwxSin/noinlineerr from 1.0.3 to 1.0.4 (golangci#5928)

* build(deps): bump golang.org/x/sys from 0.33.0 to 0.34.0 (golangci#5931)

* docs: improve debug keys documentation (golangci#5930)

* fix: panic: close of closed channel (golangci#5929)

* chore: prepare release

* docs: update GitHub Action assets

* build(deps): bump github.com/uudashr/iface from 1.4.0 to 1.4.1 (golangci#5915)

* build(deps): bump github.com/sonatard/noctx from 0.3.4 to 0.3.5 (golangci#5916)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump github.com/bombsimon/wsl/v5 from 5.0.0 to 5.1.0 (golangci#5917)

* build(deps): bump github.com/nunnatsa/ginkgolinter from 0.19.1 to 0.20.0 (golangci#5932)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump golang.org/x/sync from 0.15.0 to 0.16.0 (golangci#5934)

* build(deps): bump golang.org/x/tools from 0.34.0 to 0.35.0 (golangci#5935)

* build(deps): bump github.com/mgechev/revive from 1.10.0 to 1.11.0 (golangci#5933)

Co-authored-by: Fernandez Ludovic <[email protected]>

* build(deps): bump go-simpler.org/sloglint from 0.11.0 to 0.11.1 (golangci#5936)

* fix: panic: close of closed channel (golangci#5939)

* build(deps): bump on-headers and compression in /docs (golangci#5944)

* build(deps): bump github.com/go-viper/mapstructure/v2 from 2.3.0 to 2.4.0 (golangci#5947)

* build(deps): bump github.com/spf13/pflag from 1.0.6 to 1.0.7 (golangci#5948)

* build(deps): bump github.com/AlwxSin/noinlineerr from 1.0.4 to 1.0.5 (golangci#5949)

* build(deps): bump github.com/securego/gosec/v2 from 2.22.5 to 2.22.6 (golangci#5950)

* chore: prepare release

* docs: update documentation assets (golangci#5951)

Co-authored-by: Fernandez Ludovic <[email protected]>

* docs: update GitHub Action assets (golangci#5952)

* build(deps): bump github.com/securego/gosec/v2 from 2.22.6 to 2.22.7 (golangci#5953)

* build(deps): bump form-data from 3.0.1 to 3.0.4 in /docs (golangci#5954)

* tagliatelle: force upper case for custom initialisms (golangci#5956)

* build(deps): bump github.com/daixiang0/gci from 0.13.6 to 0.13.7 (golangci#5957)

* build(deps): bump github.com/ldez/grignotin from 0.9.0 to 0.10.0 (golangci#5958)

* build(deps): bump github.com/bombsimon/wsl/v5 from 5.1.0 to 5.1.1 (golangci#5959)

* build(deps): bump github.com/sonatard/noctx from 0.3.5 to 0.4.0 (golangci#5960)

* build(deps): bump the linter-testdata group across 3 directories with 3 updates (golangci#5964)

* chore: prepare release

* chore(deps): update dependency go to v1.24.5

---------

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ludovic Fernandez <[email protected]>
Co-authored-by: GolangCI-Lint Releaser <[email protected]>
Co-authored-by: Oleksandr Redko <[email protected]>
Co-authored-by: Matthew Gabeler-Lee <[email protected]>
Co-authored-by: kasaikou <[email protected]>
Co-authored-by: Gareth Jones <[email protected]>
Co-authored-by: Tatsuya Kyushima <[email protected]>
Co-authored-by: logica0419 <[email protected]>
Co-authored-by: Manuel Doncel Martos <[email protected]>
Co-authored-by: Chris Reeves <[email protected]>
Co-authored-by: Nikita Mironov <[email protected]>
Co-authored-by: sivchari <[email protected]>
Co-authored-by: Tom Vendolsky <[email protected]>
Co-authored-by: Cooper Benson <[email protected]>
Co-authored-by: Gabriel Augendre <[email protected]>
Co-authored-by: Manuel Doncel Martos <[email protected]>
Co-authored-by: Takeo Kasai <[email protected]>
Co-authored-by: Simon Sawert <[email protected]>
Co-authored-by: ccoVeille <[email protected]>
Co-authored-by: Alwx <[email protected]>
Co-authored-by: Ville Skyttä <[email protected]>
Co-authored-by: alaudaa-renovate[bot] <219066560+alaudaa-renovate[bot]@users.noreply.github.com>
@ruanxin
Copy link

ruanxin commented Aug 12, 2025

Hi @AlwxSin:

I just noticed your contribution while our team was trying to adopt the latest golangci-lint. First, thanks for your work on this, and I understand the intent behind it, but I’d like to offer a different perspective.

One important advantage of inline error handling is that it limits the scope of the error variable to exactly where it’s relevant. When an error is only used for a single operation, there’s no need to keep it alive in a wider scope. This helps avoid accidental reuse or confusion with unrelated errors, and keeps reasoning about the code more localized.

When inline handling is disallowed, you’re forced to declare a top-level err and reuse it for multiple unrelated calls, which can subtly reduce readability. For example:

err := doSomething()
if err != nil {
    return err
}

err = doAnotherThing()
if err != nil {
    return err
}

err = doLastThing()
if err != nil {
    return err
}

Here, a single err variable is reused across unrelated operations, and readers must mentally track its value over a longer stretch of code.

With inline checks, each error is self-contained and unambiguous:

This approach makes it immediately clear where each error came from, and the variable’s scope exactly matches its relevance.

It’s also worth noting that major style guides explicitly recommend inline error handling in certain cases:

Google Code Style – Concision
image

Uber Go Style Guide – Reduce Scope of Variables
image

For these reasons, I believe your noinlinerr is basically against those recommendations. It removes a useful language feature that often improves scope control, readability, and maintainability.

cc @ldez, enabling this linter by default really goes against widely known, established style recommendations. Could we at least mark it as not recommended by default?

@bombsimon
Copy link
Member

The linter is not enabled by default, no new linters are. If you don't agree with the style (I don't), don't enable the linter.

@ldez
Copy link
Member

ldez commented Aug 12, 2025

This linter, like other linters, is related to style.
The style is personal, so I don't want to provide any recommendations about using or not using a linter.

@ruanxin
Copy link

ruanxin commented Aug 12, 2025

Hi @bombsimon @ldez , thanks for the quick feedback.

First, I have to apologise, after checking our golangci.yaml, I realised the issue happened because we have default: all enabled and then selectively disable specific linters. That means when we upgraded, noinlineerr became active automatically.

But this actually highlights a point: our team (and I suspect many others) treat golangci-lint as a trusted guideline for recommended code style. When new linters are added, the implicit expectation is that they are widely recognized, well-accepted, and non-controversial. In this case, as I mentioned earlier, many in the Go community consider banning inline error handling a code smell rather than a style preference, as I mentioned in the Uber’s guideline, which explicitly encourages the opposite.

By including a specific linter, golangci-lint still implicitly signals that this style is worth enforcing, since golangci-lint is trusted by a huge portion of the Go community, that signal matters I believe.

However, if the position of golangci-lint is that style linters are purely neutral and not meant to guide or educate on best practices, then I understand, but that’s a different philosophy than what I think many users expect. Perhaps we’ve simply placed too much expectation on golangci-lint as a de facto style authority, but that expectation is real.

Follow-up question: if someone later proposed a linter that enforces the use of inline error handling, which directly conflicts with this noinlineerr, as it's exactly the opposite style, but is backed by reputable guidelines. Would you accept such linter? If the answer is yes, I probably will contribute to that linter 😄

@AlwxSin
Copy link
Contributor Author

AlwxSin commented Aug 12, 2025

@ruanxin I completely understand that handling errors inline is largely a matter of personal preference. Personally, I find it harder to read code where errors are inlined, because it shifts the focus from the function call itself to the error handling logic. That’s the main reason why I created this linter.

I also intentionally decided not to enable this linter by default, knowing that in existing codebases this could cause significant issues. In my view, this is a highly opinionated linter, so the decision to enable it should be left entirely to the user.

P.S.I respect the guidelines from Uber and Google, though I see them as recommendations, not mandatory rules, and I prefer a different approach in this case.

@ldez
Copy link
Member

ldez commented Aug 12, 2025

golangci-lint is "neutral" but tries not to recommend bad practices.

I would emphasize that the user guides you are quoting don't enforce the usage of error inlining.
They use terms like "Where possible" because always inlining errors is a bad practice.

I have a lot of examples where error inlining created unwanted complexity by hiding factorization or creating shadow declarations.

Always not inlining is not a bad practice; this is just a point of view.

So, I don't think we will accept a linter that forces the usage of error inlining, but you can create a plugin if you want.

https://golangci-lint.run/docs/plugins/module-plugins/

@AlwxSin
Copy link
Contributor Author

AlwxSin commented Aug 12, 2025

ok, so we agree that if a new linter is added that handles the whole topic of inlined if, your linter will be deprecated inside golangci-lint and removed.

If we agree on that, we can accept this linter.

@ruanxin ldez’s comment actually touches on exactly this point — if there is a linter that comprehensively covers all cases of inline if usage and allows enforcing or forbidding any variation, then my noinlineerr linter would become unnecessary.

@bombsimon
Copy link
Member

bombsimon commented Aug 12, 2025

But this actually highlights a point: our team (and I suspect many others) treat golangci-lint as a trusted guideline for recommended code style. When new linters are added, the implicit expectation is that they are widely recognized, well-accepted, and non-controversial.

If so, golangci-linter would only accept linters that would make sense if enabling all linters would be a recommended way. In fact, there are other considerations of not doing that, e.g. #4661 which is about duplicated/overlapping linters. As mentioned, golangci-lint is supposed to be neutral, maybe this is something that could be documented better.

I think the discussion so far has been around how to make sharable presets/sharable standards such as the eslint-config-airbinb has been for JS/TS which would promote styles you like such as Uber or Google style guide.

Some reference to previous issues/discussions where recommended/sharable configuration has been discussed:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter: new Support new linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants